cluster manager: change underlying call chain for drainConnections#17950
cluster manager: change underlying call chain for drainConnections#17950
Conversation
Signed-off-by: Jose Nino <jnino@lyft.com>
|
@mattklein123 I wanted to get this up asap to get your take. I felt that all the logic in onHostHealthFailure was what we wanted out of re: testing. The existing test obviously passes, but I am still working on a test that more explicitly makes sure that a new connection is establish for a stream even if the previous connection was not idle. I had the idea to look for how onHostHealthFailure is tested this morning so I will push an update after my morning meetings. Update: there's no easy way I can see to directly observe the behavior. The existing onHostHealthFailure test is basically the same as the existing integration test. I will merge this and continue to noodle on a better test. |
mattklein123
left a comment
There was a problem hiding this comment.
Thanks for fixing. Small comment.
/wait
Commit Message: cluster manager - change underlying call chain for drainConnections
Additional Description: previously drainConnections used
ConnPool::startDrainwhich does not guarantee that non-idle connections will not be used for new streams. This change leverages the logic in onHostHealthFailure to close idle connections and mark non-idle connections as draining so that new streams are forced onto new connections.Risk Level: low - API usage is opt in
Testing: existing tests.
Signed-off-by: Jose Nino jnino@lyft.com